Complete initial NNGv2 conversion#51
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughMigrates the crate to NNG v2 / nng-sys v0.4: adds a Params init handle and nng_init/nng_fini pairing, switches error handling to the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I have finally made it to Europe and am in the process of getting settled, so I should be able to be much more consistent in how much attention this project gets. I am not certain how you want to handle merges to |
49105b2 to
e8344be
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
nng/src/device.rs (2)
81-94:⚠️ Potential issue | 🟡 MinorSame typo in reflector function.
Line 93 also has "errror" instead of "error".
📝 Proposed fix
- unreachable!("nng_device returned with no errror"); + unreachable!("nng_device returned with no error");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/device.rs` around lines 81 - 94, In the reflector function (in nng/src/device.rs) fix the typo in the unreachable! message: replace "errror" with "error" so the unreachable!("nng_device returned with no error") uses the correct spelling; locate the unreachable! call near the nng_device invocation and update that string accordingly.
44-55:⚠️ Potential issue | 🟡 MinorTypo in unreachable message.
Line 53 has "errror" instead of "error".
📝 Proposed fix
- unreachable!("nng_device returned with no errror"); + unreachable!("nng_device returned with no error");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/device.rs` around lines 44 - 55, Fix the typo in the unreachable! message in the nng_device completion path: change the string "nng_device returned with no errror" to "nng_device returned with no error" in the block that calls nng_sys::nng_device (the code that sets rv and drops s1/s2), ensuring the unreachable! macro uses the correct spelling; refer to variables rv, s1, s2 and the nng_device call to locate the statement.nng/src/lib.rs (2)
125-160:⚠️ Potential issue | 🟠 Major
Paramsis additive, but the options API removal is still a semver break.
cargo semver-checksis already failing because this PR removes the publicnng::optionsmodule plus its traits/enums. Since the stated goal is a minimal NNGv2 compatibility pass rather than a coordinated major release, this needs compatibility shims/deprecations or an explicit major-version bump before merge.Also applies to: 162-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/lib.rs` around lines 125 - 160, The PR removed the public nng::options module and its traits/enums which breaks semver; either reintroduce a compatibility shim or perform a major version bump—fix by restoring a public options module that re-exports or wraps the new API with deprecated shims (e.g., recreate pub mod options that re-exports the old trait/enum names and implements them in terms of the new Params/additive API), mark those types/functions #[deprecated] with guidance to migrate, and ensure symbols referenced in tests/clients (the removed traits/enums and the module name options) are available so cargo semver-checks passes until a coordinated major release is done.
125-259:⚠️ Potential issue | 🟠 MajorChange thread count fields to
NonZeroI16to prevent silent wraparound.
nng_sys::nng_init_paramsuses signed 16-bit fields for thread counts. Currently,ParamsusesNonZeroU16fields and casts them toi16during conversion. This silently wraps values abovei16::MAX(32767) to negative numbers—for example, 40000 becomes -25536. The type system should enforce the constraint at construction time rather than allowing invalid values to reach the C library.Change all thread count fields in
ParamsfromNonZeroU16toNonZeroI16, and update the conversion closure tolet convert = |n: NonZeroI16| n.get();to eliminate the unnecessary cast.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/lib.rs` around lines 125 - 259, The thread-count fields on Params (num_task_threads, max_task_threads, num_expire_threads, max_expire_threads, num_poller_threads, max_poller_threads, num_resolver_threads) use NonZeroU16 which allows values > i16::MAX to silently wrap when converted; change their types to NonZeroI16, update the From<Params> for nng_sys::nng_init_params implementation to use let convert = |n: NonZeroI16| n.get(); and remove the u16->i16 cast so values map directly into nng_init_params without wraparound.
🧹 Nitpick comments (1)
nng/src/ctx.rs (1)
158-164: Inconsistent qualification style.Line 160 uses fully qualified
nng_sys::nng_err::NNG_ECLOSEDwhilenng_erris already imported. Other files (e.g.,listener.rsline 92, 241) use justnng_err::NNG_ECLOSEDafter the same import. Consider aligning for consistency.♻️ Suggested consistency fix
- rv == 0 || nng_err(rv as _) == nng_sys::nng_err::NNG_ECLOSED, + rv == 0 || nng_err(rv as _) == nng_err::NNG_ECLOSED,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/ctx.rs` around lines 158 - 164, The code in ctx.rs uses a fully qualified enum variant `nng_sys::nng_err::NNG_ECLOSED` while `nng_err` is already imported elsewhere; update the assertion in the context close path (the block that calls `nng_ctx_close(self.ctx)` inside the Drop/close logic) to use the same qualification style as other files by replacing `nng_sys::nng_err::NNG_ECLOSED` with `nng_err::NNG_ECLOSED` so it matches uses in `listener.rs` and maintains consistent imports and style across the crate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nng/CHANGELOG.adoc`:
- Line 21: Remove the personal reminder "(NOTE TO SELF: make a replacement)"
from the changelog entry "* Anything and everything related to options (NOTE TO
SELF: make a replacement)" and either delete that parenthetical entirely or
replace it with a public tracking reference (e.g., "see issue `#NNNN`" or "tracked
by TODO-ISSUE") so the entry is professional; if you prefer creation of an
issue, open one to track the options replacement work and reference its
identifier in the modified entry.
In `@nng/src/lib.rs`:
- Around line 169-171: The public struct Params is annotated with
#[non_exhaustive], which requires Rust 1.40.0 but the crate advertises MSRV
1.36.0; either remove the #[non_exhaustive] attribute from the Params
declaration to retain 1.36 compatibility, or instead update the crate MSRV
metadata (Cargo.toml rust-version / README and CI/tooling that enforce MSRV) to
1.40.0+ and adjust CI/toolchain tests accordingly; reference the Params struct
and the #[non_exhaustive] attribute when making the change.
---
Outside diff comments:
In `@nng/src/device.rs`:
- Around line 81-94: In the reflector function (in nng/src/device.rs) fix the
typo in the unreachable! message: replace "errror" with "error" so the
unreachable!("nng_device returned with no error") uses the correct spelling;
locate the unreachable! call near the nng_device invocation and update that
string accordingly.
- Around line 44-55: Fix the typo in the unreachable! message in the nng_device
completion path: change the string "nng_device returned with no errror" to
"nng_device returned with no error" in the block that calls nng_sys::nng_device
(the code that sets rv and drops s1/s2), ensuring the unreachable! macro uses
the correct spelling; refer to variables rv, s1, s2 and the nng_device call to
locate the statement.
In `@nng/src/lib.rs`:
- Around line 125-160: The PR removed the public nng::options module and its
traits/enums which breaks semver; either reintroduce a compatibility shim or
perform a major version bump—fix by restoring a public options module that
re-exports or wraps the new API with deprecated shims (e.g., recreate pub mod
options that re-exports the old trait/enum names and implements them in terms of
the new Params/additive API), mark those types/functions #[deprecated] with
guidance to migrate, and ensure symbols referenced in tests/clients (the removed
traits/enums and the module name options) are available so cargo semver-checks
passes until a coordinated major release is done.
- Around line 125-259: The thread-count fields on Params (num_task_threads,
max_task_threads, num_expire_threads, max_expire_threads, num_poller_threads,
max_poller_threads, num_resolver_threads) use NonZeroU16 which allows values >
i16::MAX to silently wrap when converted; change their types to NonZeroI16,
update the From<Params> for nng_sys::nng_init_params implementation to use let
convert = |n: NonZeroI16| n.get(); and remove the u16->i16 cast so values map
directly into nng_init_params without wraparound.
---
Nitpick comments:
In `@nng/src/ctx.rs`:
- Around line 158-164: The code in ctx.rs uses a fully qualified enum variant
`nng_sys::nng_err::NNG_ECLOSED` while `nng_err` is already imported elsewhere;
update the assertion in the context close path (the block that calls
`nng_ctx_close(self.ctx)` inside the Drop/close logic) to use the same
qualification style as other files by replacing `nng_sys::nng_err::NNG_ECLOSED`
with `nng_err::NNG_ECLOSED` so it matches uses in `listener.rs` and maintains
consistent imports and style across the crate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bbdeff3-1144-420c-a954-6b6640b1a7d8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
nng/CHANGELOG.adocnng/Cargo.tomlnng/src/addr.rsnng/src/aio.rsnng/src/ctx.rsnng/src/device.rsnng/src/dialer.rsnng/src/error.rsnng/src/lib.rsnng/src/listener.rsnng/src/options/mod.rsnng/src/options/private.rsnng/src/options/types.rsnng/src/pipe.rsnng/src/socket.rsnng/src/util.rs
💤 Files with no reviewable changes (4)
- nng/src/addr.rs
- nng/src/options/mod.rs
- nng/src/options/types.rs
- nng/src/options/private.rs
|
|
||
| === Removed === | ||
|
|
||
| * Anything and everything related to options (NOTE TO SELF: make a replacement). |
There was a problem hiding this comment.
Internal note should be removed or converted to a TODO issue.
The text (NOTE TO SELF: make a replacement) appears to be a personal reminder that shouldn't be in the public changelog.
📝 Suggested options
Either remove the note:
-* Anything and everything related to options (NOTE TO SELF: make a replacement).
+* Removed the entire `options` module and related option accessor APIs.Or create a tracking issue and reference it in the changelog entry.
Would you like me to help draft a more detailed changelog entry describing what was removed, or open an issue to track the options replacement work?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Anything and everything related to options (NOTE TO SELF: make a replacement). | |
| * Removed the entire `options` module and related option accessor APIs. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/CHANGELOG.adoc` at line 21, Remove the personal reminder "(NOTE TO SELF:
make a replacement)" from the changelog entry "* Anything and everything related
to options (NOTE TO SELF: make a replacement)" and either delete that
parenthetical entirely or replace it with a public tracking reference (e.g.,
"see issue `#NNNN`" or "tracked by TODO-ISSUE") so the entry is professional; if
you prefer creation of an issue, open one to track the options replacement work
and reference its identifier in the modified entry.
| #[non_exhaustive] | ||
| #[derive(Debug, Default)] | ||
| pub struct Params { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What Rust version stabilized the #[non_exhaustive] attribute for public structs and enums?
💡 Result:
Rust 1.40.0 stabilized the #[non_exhaustive] attribute for public structs and enums.
Citations:
- 1: Stabilize
#[non_exhaustive](RFC 2008) rust-lang/rust#64639 - 2: https://blog.rust-lang.org/2019/12/19/Rust-1.40.0/
- 3: https://releases.rs/docs/1.40.0/
- 4: https://github.com/rust-lang/rust/releases/tag/1.40.0
#[non_exhaustive] requires Rust 1.40.0 but crate advertises MSRV 1.36.0.
The #[non_exhaustive] attribute on the public Params struct is only available in Rust 1.40.0+, conflicting with the crate's documented MSRV. Either remove the attribute to maintain 1.36.0 compatibility or update the documented and tested MSRV to 1.40.0+.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/src/lib.rs` around lines 169 - 171, The public struct Params is annotated
with #[non_exhaustive], which requires Rust 1.40.0 but the crate advertises MSRV
1.36.0; either remove the #[non_exhaustive] attribute from the Params
declaration to retain 1.36 compatibility, or instead update the crate MSRV
metadata (Cargo.toml rust-version / README and CI/tooling that enforce MSRV) to
1.40.0+ and adjust CI/toolchain tests accordingly; reference the Params struct
and the #[non_exhaustive] attribute when making the change.
e8344be to
f094ba2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nng/src/device.rs (1)
53-53:⚠️ Potential issue | 🟡 MinorFix typo: "errror" → "error"
Both
unreachable!messages have the same typo.✏️ Proposed fix
- unreachable!("nng_device returned with no errror"); + unreachable!("nng_device returned with no error");Apply the same fix on line 93.
Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/device.rs` at line 53, There are two typoed panic messages using unreachable! with "nng_device returned with no errror"; locate the unreachable! invocations in nng/src/device.rs (the calls associated with the nng_device handling) and change the string to "nng_device returned with no error" in both occurrences so both panic messages are corrected and identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nng/src/lib.rs`:
- Around line 246-258: The conversion closure currently casts NonZeroU16 to i16
which overflows for values > 32767; update the From<Params> for
nng_sys::nng_init_params implementation to clamp/saturate each NonZeroU16 to
i16::MAX instead of direct casting (e.g., replace convert = |n: NonZeroU16|
n.get() as i16 with a clamp that takes min(n.get(), i16::MAX as u16) and then
casts to i16), leaving map_or(0, ...) semantics intact so out-of-range inputs
become i16::MAX; alternatively, if you prefer a stricter API change, change
Params to use NonZeroI16-backed types and map them directly to the
nng_init_params fields.
In `@nng/src/socket.rs`:
- Around line 77-86: Reformat the multi-line block around the rv2res! expansion
and the unsafe nng_fini() call to match rustfmt expectations: run cargo fmt and
ensure the block that constructs Socket (the unsafe nng_init(ptr::null()) call
inside the rv2res! expansion and the Socket { inner: Arc::new(Inner { handle:
socket, pipe_notify: RwLock::new(None) }) } structure) is formatted as rustfmt
dictates, and similarly format the standalone unsafe { nng_sys::nng_fini(); }
statement; use the rv2res! macro, nng_init, nng_fini, Socket, Inner, and
pipe_notify identifiers to locate the code to reformat.
- Around line 610-614: The Drop impl for the socket calls unsafe
nng_sys::nng_fini() before self.close(), which can free global NNG resources
while the socket still needs them; modify the Drop for the Socket (drop method)
to first call self.close() to properly release socket resources and only after
successful close call unsafe { nng_sys::nng_fini(); }, preserving the SAFETY
comment and any required unsafe block semantics so the global finalizer runs
after per-socket cleanup.
---
Outside diff comments:
In `@nng/src/device.rs`:
- Line 53: There are two typoed panic messages using unreachable! with
"nng_device returned with no errror"; locate the unreachable! invocations in
nng/src/device.rs (the calls associated with the nng_device handling) and change
the string to "nng_device returned with no error" in both occurrences so both
panic messages are corrected and identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d3535a0-82b9-40e5-b598-ec24a69cdd90
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
nng/CHANGELOG.adocnng/Cargo.tomlnng/src/addr.rsnng/src/aio.rsnng/src/ctx.rsnng/src/device.rsnng/src/dialer.rsnng/src/error.rsnng/src/lib.rsnng/src/listener.rsnng/src/options/mod.rsnng/src/options/private.rsnng/src/options/types.rsnng/src/pipe.rsnng/src/socket.rsnng/src/util.rs
💤 Files with no reviewable changes (4)
- nng/src/addr.rs
- nng/src/options/mod.rs
- nng/src/options/types.rs
- nng/src/options/private.rs
✅ Files skipped from review due to trivial changes (2)
- nng/CHANGELOG.adoc
- nng/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- nng/src/ctx.rs
- nng/src/aio.rs
- nng/src/pipe.rs
- nng/src/dialer.rs
- nng/src/error.rs
| impl From<Params> for nng_sys::nng_init_params { | ||
| fn from(params: Params) -> Self { | ||
| let convert = |n: NonZeroU16| n.get() as i16; | ||
| nng_sys::nng_init_params { | ||
| num_task_threads: params.num_task_threads.map_or(0, convert), | ||
| max_task_threads: params.max_task_threads.map_or(0, convert), | ||
| num_expire_threads: params.num_expire_threads.map_or(0, convert), | ||
| max_expire_threads: params.max_expire_threads.map_or(0, convert), | ||
| num_poller_threads: params.num_poller_threads.map_or(0, convert), | ||
| max_poller_threads: params.max_poller_threads.map_or(0, convert), | ||
| num_resolver_threads: params.num_resolver_threads.map_or(0, convert), | ||
| } | ||
| } |
There was a problem hiding this comment.
Integer overflow: NonZeroU16 values > 32767 corrupt thread counts when cast to i16.
The nng_init_params fields are i16 (per nng-sys/src/bindings.rs), but NonZeroU16 can hold values up to 65535. The cast n.get() as i16 will overflow for values > 32767, producing incorrect or negative thread counts.
🐛 Proposed fix: clamp or reject out-of-range values
Option 1: Saturate at i16::MAX
impl From<Params> for nng_sys::nng_init_params {
fn from(params: Params) -> Self {
- let convert = |n: NonZeroU16| n.get() as i16;
+ let convert = |n: NonZeroU16| n.get().min(i16::MAX as u16) as i16;
nng_sys::nng_init_params {Option 2: Change field type to Option<NonZeroI16> to make the constraint explicit in the API, preventing users from providing out-of-range values.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| impl From<Params> for nng_sys::nng_init_params { | |
| fn from(params: Params) -> Self { | |
| let convert = |n: NonZeroU16| n.get() as i16; | |
| nng_sys::nng_init_params { | |
| num_task_threads: params.num_task_threads.map_or(0, convert), | |
| max_task_threads: params.max_task_threads.map_or(0, convert), | |
| num_expire_threads: params.num_expire_threads.map_or(0, convert), | |
| max_expire_threads: params.max_expire_threads.map_or(0, convert), | |
| num_poller_threads: params.num_poller_threads.map_or(0, convert), | |
| max_poller_threads: params.max_poller_threads.map_or(0, convert), | |
| num_resolver_threads: params.num_resolver_threads.map_or(0, convert), | |
| } | |
| } | |
| impl From<Params> for nng_sys::nng_init_params { | |
| fn from(params: Params) -> Self { | |
| let convert = |n: NonZeroU16| n.get().min(i16::MAX as u16) as i16; | |
| nng_sys::nng_init_params { | |
| num_task_threads: params.num_task_threads.map_or(0, convert), | |
| max_task_threads: params.max_task_threads.map_or(0, convert), | |
| num_expire_threads: params.num_expire_threads.map_or(0, convert), | |
| max_expire_threads: params.max_expire_threads.map_or(0, convert), | |
| num_poller_threads: params.num_poller_threads.map_or(0, convert), | |
| max_poller_threads: params.max_poller_threads.map_or(0, convert), | |
| num_resolver_threads: params.num_resolver_threads.map_or(0, convert), | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/src/lib.rs` around lines 246 - 258, The conversion closure currently
casts NonZeroU16 to i16 which overflows for values > 32767; update the
From<Params> for nng_sys::nng_init_params implementation to clamp/saturate each
NonZeroU16 to i16::MAX instead of direct casting (e.g., replace convert = |n:
NonZeroU16| n.get() as i16 with a clamp that takes min(n.get(), i16::MAX as u16)
and then casts to i16), leaving map_or(0, ...) semantics intact so out-of-range
inputs become i16::MAX; alternatively, if you prefer a stricter API change,
change Params to use NonZeroI16-backed types and map them directly to the
nng_init_params fields.
| fn drop(&mut self) { | ||
| // SAFETY: This is paired with an init when the socket was created. | ||
| unsafe { nng_sys::nng_fini(); } | ||
| self.close(); | ||
| } |
There was a problem hiding this comment.
Critical: nng_fini() called before socket close will cause use-after-free.
The nng_fini() call precedes self.close(), but the socket requires NNG resources to close properly. This ordering will attempt to use NNG resources after they've been released.
🐛 Proposed fix
impl Drop for Inner {
fn drop(&mut self) {
- // SAFETY: This is paired with an init when the socket was created.
- unsafe { nng_sys::nng_fini(); }
self.close();
+ // SAFETY: This is paired with an init when the socket was created.
+ unsafe { nng_sys::nng_fini(); }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn drop(&mut self) { | |
| // SAFETY: This is paired with an init when the socket was created. | |
| unsafe { nng_sys::nng_fini(); } | |
| self.close(); | |
| } | |
| fn drop(&mut self) { | |
| self.close(); | |
| // SAFETY: This is paired with an init when the socket was created. | |
| unsafe { nng_sys::nng_fini(); } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/src/socket.rs` around lines 610 - 614, The Drop impl for the socket calls
unsafe nng_sys::nng_fini() before self.close(), which can free global NNG
resources while the socket still needs them; modify the Drop for the Socket
(drop method) to first call self.close() to properly release socket resources
and only after successful close call unsafe { nng_sys::nng_fini(); }, preserving
the SAFETY comment and any required unsafe block semantics so the global
finalizer runs after per-socket cleanup.
c480b21 to
ab1bced
Compare
This is just the minimal set of changes to get `nng-rs` compiling against NNGv2 and is not the overall changes I would like to make if I am going to do a major version bump. It does, however, mean that this library and `anng` should be able to both be compiled against the same version of `nng-sys`.
ab1bced to
e3d9b81
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
nng/src/lib.rs (2)
169-171:⚠️ Potential issue | 🟠 Major
#[non_exhaustive]raises the effective MSRV above the documented 1.36.Public
#[non_exhaustive]structs stabilized in Rust 1.40.0, but this crate still documents Rust 1.36 as supported. Either drop the attribute or update the documented/tested MSRV before shipping this API. (blog.rust-lang.org)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/lib.rs` around lines 169 - 171, The #[non_exhaustive] attribute on the public Params struct raises the effective MSRV above the documented 1.36; either remove the #[non_exhaustive] annotation from the Params struct declaration in lib.rs or update the crate MSRV metadata and tests (e.g., bump documented Rust version and rust-version/Cargo.toml and CI toolchains to >=1.40.0) so the public API is consistent with using #[non_exhaustive].
246-258:⚠️ Potential issue | 🔴 CriticalClamp or reject values above
i16::MAXbefore writingnng_init_params.The NNG init fields are
int16_t, butNonZeroU16accepts values up to 65535.n.get() as i16will therefore wrap 32768+ into negative thread counts and pass corrupted tunables intonng_init. Clamp, validate, or narrow the public type so impossible values cannot cross the FFI boundary. (nng.nanomsg.org)Minimal fix
impl From<Params> for nng_sys::nng_init_params { fn from(params: Params) -> Self { - let convert = |n: NonZeroU16| n.get() as i16; + let convert = |n: NonZeroU16| n.get().min(i16::MAX as u16) as i16; nng_sys::nng_init_params {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/lib.rs` around lines 246 - 258, The conversion currently casts NonZeroU16 values with `let convert = |n: NonZeroU16| n.get() as i16;` which will wrap values > i16::MAX into negatives; update the conversion used in the From<Params> for nng_sys::nng_init_params implementation to clamp to i16::MAX before casting (e.g. bound the u16 with .min(i16::MAX as u16) and then cast to i16) so fields like num_task_threads, max_task_threads, num_expire_threads, max_expire_threads, num_poller_threads, max_poller_threads, and num_resolver_threads never become negative across the FFI boundary.nng/src/socket.rs (1)
610-614:⚠️ Potential issue | 🔴 CriticalCall
self.close()beforenng_fini().After
nng_fini(), the application must not make any more NNG calls exceptnng_init().self.close()invokesnng_socket_close, so the current order finalizes the library and then immediately calls back into it. Swap the two operations. (nng.nanomsg.org)Minimal fix
impl Drop for Inner { fn drop(&mut self) { - // SAFETY: This is paired with an init when the socket was created. - unsafe { - nng_sys::nng_fini(); - } self.close(); + // SAFETY: This is paired with an init when the socket was created. + unsafe { + nng_sys::nng_fini(); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nng/src/socket.rs` around lines 610 - 614, The nng library finalization (nng_fini) is currently called before closing the socket, which is unsafe because self.close() (which calls nng_socket_close) must not be invoked after nng_fini; swap the calls so that self.close() is invoked first and then the unsafe { nng_sys::nng_fini(); } is executed, preserving the existing SAFETY comment pairing with socket initialization and ensuring no NNG calls occur after finalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nng/src/lib.rs`:
- Around line 172-203: The intra-doc links incorrectly reference `Init` (which
doesn't exist) in the comments for `num_task_threads`, `num_expire_threads`, and
`num_poller_threads`; update those links to point to the `Params` type instead
(change `Init::max_task_threads` → `Params::max_task_threads`,
`Init::max_expire_threads` → `Params::max_expire_threads`, and
`Init::max_poller_threads` → `Params::max_poller_threads`) so the documentation
links resolve to the correct symbols associated with the `Params` struct/fields.
In `@nng/src/socket.rs`:
- Around line 694-697: The call to nng_sys::nng_socket_raw currently ignores its
return code and only inspects the mutable raw boolean, which masks failures;
capture the c_int return value from nng_socket_raw(socket.inner.handle, &mut
raw) and check it (e.g., ensure it is 0 / NNG_OK), returning or converting the
real error instead of proceeding to produce a CookedSocketError when the socket
handle is invalid/closed; update the code around socket.inner.handle, the raw
variable, and the CookedSocketError path to surface or assert on the actual
nng_socket_raw failure.
- Around line 75-86: Move the NNG initialization before any socket open calls
and centralize that logic into a shared helper (e.g., ensure_nng_initialized or
init_nng_once) used by both Socket::new and RawSocket::new so nng_init is always
called exactly once before any nng_* operations; update Inner::drop to only call
nng_fini when the shared init path actually initialized NNG (or rely on an
atomic refcount/Once to balance fini), and modify the RawSocket::new path to
invoke the shared initializer rather than skipping it. Also handle and propagate
the return value of nng_socket_raw (check its nng return code, e.g., map
non-zero to an Err) instead of assuming it always succeeds so errors like
NNG_ECLOSED are surfaced.
---
Duplicate comments:
In `@nng/src/lib.rs`:
- Around line 169-171: The #[non_exhaustive] attribute on the public Params
struct raises the effective MSRV above the documented 1.36; either remove the
#[non_exhaustive] annotation from the Params struct declaration in lib.rs or
update the crate MSRV metadata and tests (e.g., bump documented Rust version and
rust-version/Cargo.toml and CI toolchains to >=1.40.0) so the public API is
consistent with using #[non_exhaustive].
- Around line 246-258: The conversion currently casts NonZeroU16 values with
`let convert = |n: NonZeroU16| n.get() as i16;` which will wrap values >
i16::MAX into negatives; update the conversion used in the From<Params> for
nng_sys::nng_init_params implementation to clamp to i16::MAX before casting
(e.g. bound the u16 with .min(i16::MAX as u16) and then cast to i16) so fields
like num_task_threads, max_task_threads, num_expire_threads, max_expire_threads,
num_poller_threads, max_poller_threads, and num_resolver_threads never become
negative across the FFI boundary.
In `@nng/src/socket.rs`:
- Around line 610-614: The nng library finalization (nng_fini) is currently
called before closing the socket, which is unsafe because self.close() (which
calls nng_socket_close) must not be invoked after nng_fini; swap the calls so
that self.close() is invoked first and then the unsafe { nng_sys::nng_fini(); }
is executed, preserving the existing SAFETY comment pairing with socket
initialization and ensuring no NNG calls occur after finalization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 878fe6d4-18e9-483c-a4fe-3be01deada6d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
nng/CHANGELOG.adocnng/Cargo.tomlnng/src/addr.rsnng/src/aio.rsnng/src/ctx.rsnng/src/device.rsnng/src/dialer.rsnng/src/error.rsnng/src/lib.rsnng/src/listener.rsnng/src/options/mod.rsnng/src/options/private.rsnng/src/options/types.rsnng/src/pipe.rsnng/src/socket.rsnng/src/util.rs
💤 Files with no reviewable changes (4)
- nng/src/addr.rs
- nng/src/options/mod.rs
- nng/src/options/private.rs
- nng/src/options/types.rs
✅ Files skipped from review due to trivial changes (3)
- nng/CHANGELOG.adoc
- nng/Cargo.toml
- nng/src/aio.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- nng/src/device.rs
- nng/src/error.rs
- nng/src/pipe.rs
| /// The number of threads to use for tasks. | ||
| /// | ||
| /// These tasks are principally for callback completion. This number cannot | ||
| /// exceed [`Init::max_task_threads`]. | ||
| pub num_task_threads: Option<NonZeroU16>, | ||
|
|
||
| /// The maximum number of threads to use for tasks. | ||
| /// | ||
| /// These tasks are principally used for callback completion. This value can | ||
| /// be used to provide an upper limit while still allowing the number of | ||
| /// task threads to be dynamically calculated. | ||
| pub max_task_threads: Option<NonZeroU16>, | ||
|
|
||
| /// The number of threads used for expiring operations. | ||
| /// | ||
| /// Using a larger value will reduce contention on some common locks, and | ||
| /// may improve performance. This number cannot exceed | ||
| /// [`Init::max_expire_threads`]. | ||
| pub num_expire_threads: Option<NonZeroU16>, | ||
|
|
||
| /// The maximum number of threads used for expiring operations. | ||
| /// | ||
| /// Using a larger value will reduce contention on some common locks, and | ||
| /// may improve performance. This can be used to provide an upper limit | ||
| /// while still allowing a dynamic count. | ||
| pub max_expire_threads: Option<NonZeroU16>, | ||
|
|
||
| /// The number of threads to use for performing I/O. | ||
| /// | ||
| /// Not all configurations will support this. Cannot exceed | ||
| /// [`Init::max_poller_threads`]. | ||
| pub num_poller_threads: Option<NonZeroU16>, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
fd -t f "lib.rs" | head -20Repository: nanomsg/nng-rs
Length of output: 107
🏁 Script executed:
# Find the nng crate's lib.rs specifically
find . -path "*/nng/src/lib.rs" -type fRepository: nanomsg/nng-rs
Length of output: 74
🏁 Script executed:
# Get the file and check its size
wc -l ./nng/src/lib.rsRepository: nanomsg/nng-rs
Length of output: 78
🏁 Script executed:
# Read lines 172-203 and surrounding context (lines 160-220 for context)
sed -n '160,220p' ./nng/src/lib.rs | cat -nRepository: nanomsg/nng-rs
Length of output: 2953
🏁 Script executed:
# Search for type definitions of Init and Params
rg "^(pub\s+)?(struct|enum)\s+(Init|Params)" ./nng/src/lib.rs -A 2Repository: nanomsg/nng-rs
Length of output: 133
🏁 Script executed:
# Check if Init type is defined in lib.rs
rg "^(pub\s+)?(struct|type|enum)\s+Init" ./nng/src/lib.rsRepository: nanomsg/nng-rs
Length of output: 40
🏁 Script executed:
# Also check if Init is imported from elsewhere or if it's mentioned anywhere
rg "\bInit\b" ./nng/src/lib.rs | head -20Repository: nanomsg/nng-rs
Length of output: 176
🏁 Script executed:
# Verify the complete context around the Params struct definition
sed -n '160,210p' ./nng/src/lib.rs | cat -nRepository: nanomsg/nng-rs
Length of output: 2535
Fix the intra-doc links to reference Params instead of Init.
Init is not defined in this module. The doc links at lines 176, 190, and 203 should reference Params::max_task_threads, Params::max_expire_threads, and Params::max_poller_threads respectively.
Suggested doc fix
- /// exceed [`Init::max_task_threads`].
+ /// exceed [`Params::max_task_threads`].
...
- /// [`Init::max_expire_threads`].
+ /// [`Params::max_expire_threads`].
...
- /// [`Init::max_poller_threads`].
+ /// [`Params::max_poller_threads`].📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// The number of threads to use for tasks. | |
| /// | |
| /// These tasks are principally for callback completion. This number cannot | |
| /// exceed [`Init::max_task_threads`]. | |
| pub num_task_threads: Option<NonZeroU16>, | |
| /// The maximum number of threads to use for tasks. | |
| /// | |
| /// These tasks are principally used for callback completion. This value can | |
| /// be used to provide an upper limit while still allowing the number of | |
| /// task threads to be dynamically calculated. | |
| pub max_task_threads: Option<NonZeroU16>, | |
| /// The number of threads used for expiring operations. | |
| /// | |
| /// Using a larger value will reduce contention on some common locks, and | |
| /// may improve performance. This number cannot exceed | |
| /// [`Init::max_expire_threads`]. | |
| pub num_expire_threads: Option<NonZeroU16>, | |
| /// The maximum number of threads used for expiring operations. | |
| /// | |
| /// Using a larger value will reduce contention on some common locks, and | |
| /// may improve performance. This can be used to provide an upper limit | |
| /// while still allowing a dynamic count. | |
| pub max_expire_threads: Option<NonZeroU16>, | |
| /// The number of threads to use for performing I/O. | |
| /// | |
| /// Not all configurations will support this. Cannot exceed | |
| /// [`Init::max_poller_threads`]. | |
| pub num_poller_threads: Option<NonZeroU16>, | |
| /// The number of threads to use for tasks. | |
| /// | |
| /// These tasks are principally for callback completion. This number cannot | |
| /// exceed [`Params::max_task_threads`]. | |
| pub num_task_threads: Option<NonZeroU16>, | |
| /// The maximum number of threads to use for tasks. | |
| /// | |
| /// These tasks are principally used for callback completion. This value can | |
| /// be used to provide an upper limit while still allowing the number of | |
| /// task threads to be dynamically calculated. | |
| pub max_task_threads: Option<NonZeroU16>, | |
| /// The number of threads used for expiring operations. | |
| /// | |
| /// Using a larger value will reduce contention on some common locks, and | |
| /// may improve performance. This number cannot exceed | |
| /// [`Params::max_expire_threads`]. | |
| pub num_expire_threads: Option<NonZeroU16>, | |
| /// The maximum number of threads used for expiring operations. | |
| /// | |
| /// Using a larger value will reduce contention on some common locks, and | |
| /// may improve performance. This can be used to provide an upper limit | |
| /// while still allowing a dynamic count. | |
| pub max_expire_threads: Option<NonZeroU16>, | |
| /// The number of threads to use for performing I/O. | |
| /// | |
| /// Not all configurations will support this. Cannot exceed | |
| /// [`Params::max_poller_threads`]. | |
| pub num_poller_threads: Option<NonZeroU16>, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/src/lib.rs` around lines 172 - 203, The intra-doc links incorrectly
reference `Init` (which doesn't exist) in the comments for `num_task_threads`,
`num_expire_threads`, and `num_poller_threads`; update those links to point to
the `Params` type instead (change `Init::max_task_threads` →
`Params::max_task_threads`, `Init::max_expire_threads` →
`Params::max_expire_threads`, and `Init::max_poller_threads` →
`Params::max_poller_threads`) so the documentation links resolve to the correct
symbols associated with the `Params` struct/fields.
| rv2res!(rv, { | ||
| // SAFETY: Null pointers are valid. | ||
| unsafe { | ||
| nng_sys::nng_init(ptr::null()); | ||
| } | ||
| Socket { | ||
| inner: Arc::new(Inner { | ||
| handle: socket, | ||
| pipe_notify: RwLock::new(None) | ||
| }) | ||
| pipe_notify: RwLock::new(None), | ||
| }), | ||
| } | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the specific lines mentioned
find . -name "socket.rs" -path "*/nng/src/*" -type fRepository: nanomsg/nng-rs
Length of output: 77
🏁 Script executed:
# Get the code at lines 75-86
head -100 nng/src/socket.rs | tail -30Repository: nanomsg/nng-rs
Length of output: 1087
🏁 Script executed:
# Also check the drop implementation around lines 609-615
sed -n '600,625p' nng/src/socket.rsRepository: nanomsg/nng-rs
Length of output: 921
🏁 Script executed:
# And check the TryFrom implementation around lines 694-697
sed -n '685,705p' nng/src/socket.rsRepository: nanomsg/nng-rs
Length of output: 535
🏁 Script executed:
# Look for RawSocket::new implementation
rg -n "impl.*RawSocket" nng/src/socket.rs -A 30Repository: nanomsg/nng-rs
Length of output: 2796
🏁 Script executed:
# Check the overall structure to understand init/fini placement
sed -n '1,100p' nng/src/socket.rsRepository: nanomsg/nng-rs
Length of output: 3682
🌐 Web query:
NNG nng_init documentation lifecycle requirements ordering
💡 Result:
NNG (nanomsg-next-generation) requires explicit library initialization using nng_init before using any other NNG interfaces or functions. This initializes necessary subsystems like platform, task queue, reaper, AIO, TLS, and transports in dependency order. The function uses reference counting: subsequent calls increment the counter without re-initializing; it returns thread-safely via a lock. Lifecycle: After use, call nng_fini to decrement the reference counter. When it reaches zero, it finalizes subsystems in reverse order, closes all open sockets, drains pending operations, and releases resources. Requirements: Call nng_init once (or more for refcounting) at program start before creating sockets (e.g., nng_pair0_open), dialers/listeners (nng_dial, nng_listen). No other strict ordering beyond init before socket operations. Sockets, dialers, listeners have their own creation order: create socket first, then dial/listen on it. Close in reverse: close dialers/listeners/sockets before nng_fini. Reference counting manages object lifetimes; reaper handles async cleanup. In NNG 2.x migration notes, explicit init is newly required (not in 1.x). Official docs are sparse on nng_init manpage (appears draft-only), but confirmed in source analysis and migration guide.
Citations:
- 1: https://nng.nanomsg.org/ref/api/init.html
- 2: https://github.com/nanomsg/nng/blob/main/docs/ref/migrate/nng1.md
- 3: https://deepwiki.com/nanomsg/nng/2.6-initialization-and-lifecycle
- 4: https://github.com/nanomsg/nng/blob/55925438/docs/ref/migrate/nng1.md
🌐 Web query:
nng_init nng_fini pairing NNG 2.0 documentation
💡 Result:
In NNG 2.0 (development branch, latest prerelease v2.0.0-alpha.7 as of 2026-01), applications must explicitly call nng_init before using any other NNG functions to initialize the library's global subsystems (e.g., task queues, AIO, reaper). This is a change from NNG 1.x where it was optional. nng_init may accept parameters to tune thread counts (num_task_threads, max_task_threads, etc.). Each call increments a reference counter; pair it with nng_fini which decrements it. The final nng_fini (counter reaches zero) shuts down subsystems orderly: closes sockets, drains AIO/taskq/reaper, finalizes in reverse init order. After nng_fini, no other NNG calls except nng_init are allowed. No special "pairing" is required beyond balanced calls in multi-threaded or forked environments—reference counting handles it. After nng_fini, reinitialize with nng_init if needed. The pair protocol (nng_pair0_open, nng_pair1_open, etc.) is unrelated to init/fini; it's for creating peer-to-peer sockets (one-to-one, with v1 polyamorous mode deprecated). Official docs are at https://nng.nanomsg.org/ref/api/init.html (DRAFT for 2.0). Full API in <nng/nng.h>. NNG 2.0 requires explicit init (migration guide: https://github.com/nanomsg/nng/blob/main/docs/ref/migrate/nng1.md). Source shows public wrappers around internal nni_init/fini with refcounting.
Citations:
- 1: https://nng.nanomsg.org/ref/api/init.html
- 2: https://deepwiki.com/nanomsg/nng/2.6-initialization-and-lifecycle
- 3: https://github.com/nanomsg/nng/blob/main/include/nng/nng.h
- 4: https://github.com/nanomsg/nng
- 5: https://github.com/nanomsg/nng/blob/55925438/docs/ref/xref.md
- 6: https://www.nitolmotors.com.bd
- 7: https://github.com/nanomsg/nng/blob/55925438/docs/ref/migrate/nng1.md
- 8: https://github.com/nanomsg/nng/blob/main/src/nng.c
🌐 Web query:
nng_socket_raw return value status code NNG documentation
💡 Result:
The nng_socket_raw function in the NNG library returns whether a given socket is in "raw" mode (true) or "cooked" (normal protocol) mode (false). It stores the result in the bool pointer provided as the second argument. Signature (from NNG header): int nng_socket_raw(nng_socket id, bool *rawp); Return value: 0 on success, non-zero NNG errno on error (e.g., NNG_ECLOSED if socket invalid). Common errors (from similar NNG functions like nng_socket_id, nng_req0_open): NNG_ECLOSED (socket not open), NNG_EINVAL (invalid args). Raw mode sockets bypass protocol state machines for direct header access, used in proxies or nng_device. Most apps use cooked mode. Official docs reference it in API overviews, e.g., https://nng.nanomsg.org/man/v1.10.0/ and GitHub nng.h.
Citations:
- 1: https://nng.nanomsg.org/man/v1.10.0/index.html
- 2: https://github.com/nanomsg/nng/blob/main/include/nng/nng.h
- 3: https://github.com/nanomsg/nng/blob/55925438/docs/ref/api/sock.md
- 4: https://nng.nanomsg.org/man/v1.3.0/nng_req_open.3.html
Move nng_init before socket creation and share that initialization path with RawSocket::new.
NNG 2.x documentation requires nng_init to be called first, before any other NNG operations. Currently, nng_init is called after nng_*_open succeeds, violating this contract. Additionally, RawSocket::new creates sockets without calling nng_init, but Inner::drop calls nng_fini unconditionally for all instances, creating an imbalance where some sockets have unmatched finalization calls. Move nng_init before the open call and refactor both Socket::new and RawSocket::new to use a shared initialization path. Also, check the return value of nng_socket_raw (line 696) as it can fail with errors like NNG_ECLOSED for invalid sockets. (nng.nanomsg.org)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/src/socket.rs` around lines 75 - 86, Move the NNG initialization before
any socket open calls and centralize that logic into a shared helper (e.g.,
ensure_nng_initialized or init_nng_once) used by both Socket::new and
RawSocket::new so nng_init is always called exactly once before any nng_*
operations; update Inner::drop to only call nng_fini when the shared init path
actually initialized NNG (or rely on an atomic refcount/Once to balance fini),
and modify the RawSocket::new path to invoke the shared initializer rather than
skipping it. Also handle and propagate the return value of nng_socket_raw (check
its nng return code, e.g., map non-zero to an Err) instead of assuming it always
succeeds so errors like NNG_ECLOSED are surfaced.
| // SAFETY: The handle and pointer are both valid. | ||
| unsafe { | ||
| nng_sys::nng_socket_raw(socket.inner.handle, &mut raw); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 750 nng/src/socket.rs | tail -n 100Repository: nanomsg/nng-rs
Length of output: 3679
🏁 Script executed:
# Find nng_socket_raw definition in bindings
rg "nng_socket_raw" --type rust -A 5Repository: nanomsg/nng-rs
Length of output: 712
Check the return value from nng_socket_raw() to detect invalid or closed sockets.
nng_socket_raw() returns an error code (c_int), but the current code ignores it and only checks the raw boolean. When the socket handle is invalid or closed, nng_socket_raw() fails, leaving raw at false and masking the actual error behind a CookedSocketError (incorrect socket mode). Capture the return value and either surface the real error or assert that the call succeeded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nng/src/socket.rs` around lines 694 - 697, The call to
nng_sys::nng_socket_raw currently ignores its return code and only inspects the
mutable raw boolean, which masks failures; capture the c_int return value from
nng_socket_raw(socket.inner.handle, &mut raw) and check it (e.g., ensure it is 0
/ NNG_OK), returning or converting the real error instead of proceeding to
produce a CookedSocketError when the socket handle is invalid/closed; update the
code around socket.inner.handle, the raw variable, and the CookedSocketError
path to surface or assert on the actual nng_socket_raw failure.
Welcome back :-) This is nice starting point to get The code changes look reasonable to me to get it built with v2 and you can follow up what you planned. |
|
Hello @neachdainn. Can I support somehow to get this PR forward? |
|
@flxo I assumed that either I would get an email saying one of you two approved it or one of you would trigger the merge, so it seems we were actually just waiting on each other. I've gone ahead and merged it and a hopefully-final-but-we-will see PR will be coming this weekend. |
Oh - ok. Sorry I wasn't aware of that and appreciate you brave click :-). Catching up on my comment on this MR I created #54 which should give you CI runs for |
This is just the minimal set of changes to get
nng-rscompiling against NNGv2 and is not the overall changes I would like to make if I am going to do a major version bump. It does, however, mean that this library andanngshould be able to both be compiled against the same version ofnng-sys.Summary by CodeRabbit
Breaking Changes
New Features
Behavior Changes